Remove support for 'discard' element
Categories
(Core :: SVG, task)
Tracking
()
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
(Keywords: dev-doc-complete)
Attachments
(2 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
phab-bot
:
approval-mozilla-beta+
|
Details | Review |
We recently landed a patch to add experimental support for the SVG <discard>
element, partly on the hypothesis that doing so might prompt Chromium engineers to add back their historical implementation of that element (if their removal was largely due to lack-of-support-in-other-browsers and if adding it back was not to much trouble).
However, since then:
(1) we ran across a bug in our experimental implementation; this bug indicated that there's some added complexity in making our implementation robust, and it prompted us to disable the feature (in bug 1954608).
(2) we ran across https://github.com/w3c/svgwg/issues/717#issue-470289163 where Chromium implementors requested to remove the feature from the spec when they un-shipped the feature -- they expressed additional reasons that they opted to remove the feature beyond lack-of-support-in-other-browsers, and they seem disinclined to add the feature back, given that the cost/benefit is not particularly favorable. (They note that the complexity & perf cost was nontrivial, and usage of the feature was ~zero, and it doesn't add much in the way of utility aside from the potential for SVG authors to reduce memory usage in long-running unscripted SVG animations.)
(3) @zcorpan closed the loop on the issue referenced in (2), removing the feature from the SVG spec.
Given the above (particularly (2)), it appears that the the benefits of this feature are not worth the maintenance burden & increased attack-surface cost of having this feature. Let's remove our implementation.
Comment 1•2 months ago
|
||
Added dev-doc-needed
. MDN will need some cleanup if this happens.
Assignee | ||
Comment 2•1 month ago
|
||
See reasoning in https://bugzilla.mozilla.org/show_bug.cgi?id=1958839#c0
This feature was already off by default. This patch is a squashed backout of
the following 6 changesets, listed in reverse chronological order:
https://hg.mozilla.org/mozilla-central/rev/efb626444437
https://hg.mozilla.org/mozilla-central/rev/ae8af0edf737
https://hg.mozilla.org/mozilla-central/rev/dd3fac9da2c1
https://hg.mozilla.org/mozilla-central/rev/d5e958d3caae
https://hg.mozilla.org/mozilla-central/rev/acdd71cd851b
https://hg.mozilla.org/mozilla-central/rev/86f62a648ef0
Fortunately none of the backouts needed manual merge-conflict-resolution,
so this is an entirely mechanical backout patch.
Updated•1 month ago
|
Assignee | ||
Comment 4•1 month ago
|
||
(Once we've landed on central, we should uplift to beta139 as well, since our preffed-off <discard>
implementation inadvertently exposed a stub version of the SVGDiscardElement
interface, which is a bit of a wart & is potentially confusing, per bug 1964009 and bug 1957980 and bug 1959097.)
Assignee | ||
Comment 10•25 days ago
|
||
We recently added experimental support for this element only recently, and it
was never enabled by default on release.
Our implementation has been preffed-off on all branches for a little while via
bug 1954608; and this patch removes the implementation entirely, for reasons
discussed in https://bugzilla.mozilla.org/show_bug.cgi?id=1958839#c0
This patch is a squashed backout of the following 6 changesets, listed in
reverse chronological order:
https://hg.mozilla.org/mozilla-central/rev/efb626444437
https://hg.mozilla.org/mozilla-central/rev/ae8af0edf737
https://hg.mozilla.org/mozilla-central/rev/dd3fac9da2c1
https://hg.mozilla.org/mozilla-central/rev/d5e958d3caae
https://hg.mozilla.org/mozilla-central/rev/acdd71cd851b
https://hg.mozilla.org/mozilla-central/rev/86f62a648ef0
Fortunately none of the backouts needed manual merge-conflict-resolution,
so this is an entirely mechanical backout patch.
Original Revision: https://phabricator.services.mozilla.com/D247668
Updated•25 days ago
|
Comment 11•25 days ago
|
||
firefox-beta Uplift Approval Request
- User impact if declined: Low; we just expose a "SVGDiscardElement" interface for a feature that's actually turned off (see bug 1964009) and that we don't intend to ship. It's conceivable that sites might get confused by the presence of that interface and might think it works or use it in a weird way.
- Code covered by automated testing: yes
- Fix verified in Nightly: yes
- Needs manual QE test: no
- Steps to reproduce for manual QE testing: N/A
- Risk associated with taking this patch: Minimal; this is just a rollout of backout patches for a feature that we never shipped and that we've opted to remove
- Explanation of risk level: Minimal
- String changes made/needed: None
- Is Android affected?: yes
Updated•24 days ago
|
Updated•24 days ago
|
Updated•24 days ago
|
Updated•24 days ago
|
Comment 12•24 days ago
|
||
uplift |
Comment 13•10 days ago
|
||
FF139 Docs work for this done/tracked in https://github.com/mdn/content/issues/39618
Description
•